-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix daemonset does not generate manifest #434
Conversation
I am not sure why linting fail.. It works when I run loki with tanka. Please take a look at this as it is breaking loki tanka installation |
Hello anyone looking at this? This PR will fix issue the lib implementation in loki. Thank you |
@Duologic would you be able to review this? It's breaking the lib implementaion in loki. I use this code and it works. I checked the linting error and it doesn't tell much why it is error. It shouldn't be error because it works when I run with loki |
Can you make a minimal reproducible example that fails before and succeeds after this fix? |
Before code change,
It seems this line is referring to the generated library instead of the old ksonnet util lib. CMIIW |
Your example contains many elements that make it hard to determine the cause. A minimal reproducible example should help us get to the root of the problem. |
The Loki library has locked the ksonnet-util version: https://github.com/grafana/loki/blob/b29bc19/production/ksonnet/loki/jsonnetfile.lock.json#L24-L33 I just did a quick run with this version and also get an error, imo the Loki library should not lock versions like that. |
CMIIW the lock file exist for any library used in jsonnetfile.json. It is the concept for using jb (jsonnet bundler) To see the version used, refer to jsonnetfile.json Loki is using ksonnet-lib master version |
I cannot reproduce with above instructions. I've tested this with this setup: https://github.com/Duologic/temp-debug-ksonnet-util/tree/1ebd3166c2b4d12e7da0c8f8d7b5a014eab54acb Please have a look at your project jsonnetfile.lock.json to ensure it matches jsonnet-libs master. |
To preface this comment, I have only started using Jsonnet in the last day, so I apologise if this comment is incorrect or irrelevant. That said, I was having the same issue as @c0ffeec0der - I have been trying to generate this DaemonSet manifest using the Tanka command // environments/default/main.jsonnet
(import 'github.com/grafana/jsonnet-libs/ksonnet-util/kausal.libsonnet') +
{
_config:: {
example: {
name: 'example',
image: 'example/image:latest',
},
},
local c = $._config,
local container = $.core.v1.container,
local daemonSet = $.apps.v1.daemonSet,
local port = $.core.v1.containerPort,
manifest: {
daemonSet: daemonSet.new(c.example.name, containers=[
container.new(c.example.name, c.example.image)
+ container.withPorts(
port.newNamed(12345, 'example')
),
]),
},
} Using this manifest would result in the following error:
I then tried using the modified code from @c0ffeec0der, which resulted in the successful generation of the DaemonSet manifest. However, I remembered that my // lib/k.libsonnet
(import 'github.com/jsonnet-libs/k8s-alpha/1.18/main.libsonnet') +
(import 'github.com/jsonnet-libs/k8s-alpha/1.18/extensions/kausal-shim.libsonnet') I removed @c0ffeec0der's changes to // lib/k.libsonnet
+ (import 'github.com/jsonnet-libs/k8s-alpha/1.18/main.libsonnet')
- (import 'github.com/jsonnet-libs/k8s-alpha/1.18/main.libsonnet') +
- (import 'github.com/jsonnet-libs/k8s-alpha/1.18/extensions/kausal-shim.libsonnet') ...and I could successfully generate the DaemonSet manifest! ❯ tk show environments/default
apiVersion: apps/v1
kind: DaemonSet
metadata:
name: example
namespace: default
spec:
minReadySeconds: 10
selector:
matchLabels:
name: example
template:
metadata:
labels:
name: example
spec:
containers:
- image: example/image:latest
imagePullPolicy: IfNotPresent
name: example
ports:
- containerPort: 12345
name: example
tolerations:
- effect: NoSchedule
operator: Exists
updateStrategy:
type: RollingUpdate Not really sure how or why this worked (as I said, I'm only at the beginning of my journey into Jsonnet), but I thought this might be useful to others who know a bit more than I do 🙂 |
Aha, that seems to be it, the kausal-shim is now integrated into ksonnet-util. Removing that is the right choice. It looks like the docs have recently (8 days ago) been updated, it doesn't reference the kausal-shim anymore. @c0ffeec0der Can you confirm? @snoord Thank your for that insight. |
I'm considering this solved. |
Fix issue #433